Skip to content

(Naga) Implement OpSpecConstantOp for the SPIR-V frontend #8308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

(Naga) Implement OpSpecConstantOp for the SPIR-V frontend #8308

wants to merge 3 commits into from

Conversation

hasenbanck
Copy link
Contributor

@hasenbanck hasenbanck commented Oct 6, 2025 •
edited
Loading

Description

Implement OpSpecConstantOp for the SPIR-V frontend.

This implementation solves all my problems that I had with SLANGC output.
But I couldn't implement all operations. The following are missing:

  • Op::VectorShuffle, Op::CompositeExtract, Op::CompositeInsert => All work on non-scalars, which I can't get to work.
  • Op::QuantizeToF16 => We don't support QuantizeToF16 in the SPIR-V frontend yet. I think the constant evaluator might also have problems with it.

We also properly convert implicit type conversions to explicit type conversions, convert signed shifts to unsigned shifts and also optimize common type conversion patterns produced by the GLSL compiler or SLANG.

Testing

I added two snapshot tests. One based on GLSL code and one in pure SPVASM, that tests more complex chained OpSpecConstantOps.

Squash or Rebase?

Rebase

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@hasenbanck hasenbanck force-pushed the spec-constant-op branch 2 times, most recently from 4bf5d79 to e857d2b Compare October 6, 2025 15:30
@hasenbanck hasenbanck marked this pull request as ready for review October 7, 2025 04:50
@hasenbanck hasenbanck force-pushed the spec-constant-op branch 4 times, most recently from 27adeee to 7f7910b Compare October 7, 2025 13:59
@hasenbanck
Copy link
Contributor Author

I added a second commit to the PR that removes a workaround and instead uses the same approach as the GLSL frontend, by properly using the ConstantEvaluatorto evaluate the constant expressions. This is a more invasive change, so I decide it to add as a second commit, but could be spun into it's own PR if the reviewer think that's the better option.

This would enable us in a future PR to extend the ConstantEvaluator to have a mode for SPIR-V, that takes care for the corner cases that compiler could create (like the corner cases by the GLSL compiler described above).

@hasenbanck
Copy link
Contributor Author

hasenbanck commented Oct 8, 2025 •
edited
Loading

Since no reviewer has assigned the PR to them, I took the liberty and added a third commit that polishes the implementation:

Implicit type conversions are now made explicit, so that we can now properly convert OpSpecConstantOp created by GLSL, that contain implicit type conversion properly into stricter forms, so that WGSL can validate and evaluate the overrides correctly.

We now also properly use the best available name for the override and also detect and optimize type conversion patterns that compiler like GLSL love to create. This improves debuggability of the produced shaders greatly.

I will update the PR text accordingly.

@hasenbanck
(Naga) Handle implicit casting of operators in SPIR-V frontend
f1c1b50 
We also now also try to select the best name for the override
We also handle optimization of common conversion pattern to `As` instructions and with a proper, derived name.

This makes the constants of the final shader of a GLSL compiled SPIR-V highly readable.
@jimblandy jimblandy self-assigned this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jimblandy jimblandy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hasenbanck @jimblandy